Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly handle a cached solution having frozen generated documents #59723

Conversation

jasonmalinowski
Copy link
Member

@jasonmalinowski jasonmalinowski commented Feb 24, 2022

When we are synchronizing solutions, the previous assumption in this code was incorrect -- we absolutely can end up with a frozen generated document in CurrentSolution, since the remote workspace caches things there. We're going to just run with that and consider that normal, so we'll fix up the code to support it cleanly.

Code review note: commits have other comments that might explain some more detail, but isn't critical to understanding the main change.

Fixes #57082
Fixes #56998

…ngCompilationTracker

This is just a simple conversion of the field to a property.
In this particular code path, we were passing null for the frozen
source generated document state when we could have passed it along.
The original logic here was frozen source generated documents only
normally appear when somebody calls
ITextSnapshot.GetOpenDocumentInCurrentContextWithChanges, since we need
to ensure that there is a generated document with that exact snapshot
for the feature to work. We should never see that in a "primary"
solution in any way. However, solution sync uses CurrentSolution as
a cache for the previously synced complete solution, which meant
we'd still end up calling this anyways.

Although we could (and probably should) avoid caching these special
solutions entirely, this avoids any surprises here and makes things
behave like normal, and avoids odd cases where the CompilationTrackers
are out of sync with this field.

This partially addresses dotnet#57082
@jasonmalinowski jasonmalinowski requested a review from a team as a code owner February 24, 2022 00:25
@jasonmalinowski jasonmalinowski marked this pull request as draft February 24, 2022 00:26
When we are synchronizing solutions, the previous assumption in this
code was incorrect -- we absolutely can end up with a frozen generated
document in CurrentSolution, since the remote workspace caches things
there. We're going to just run with that and consider that normal, so
we'll fix up the code to support it cleanly.

Fixes dotnet#57082
Fixes dotnet#56998
@jasonmalinowski jasonmalinowski self-assigned this Feb 24, 2022
@jasonmalinowski jasonmalinowski force-pushed the fix-solution-sync-with-frozen-generated-documents branch from 5556644 to 314bfbc Compare February 24, 2022 02:28
@jasonmalinowski jasonmalinowski marked this pull request as ready for review February 24, 2022 02:28
Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me

@jasonmalinowski jasonmalinowski merged commit 97fc0d2 into dotnet:main Feb 25, 2022
@jasonmalinowski jasonmalinowski deleted the fix-solution-sync-with-frozen-generated-documents branch February 25, 2022 18:37
@ghost ghost added this to the Next milestone Feb 25, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P2 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants